feat(resources): browsable room finder with availability and dropdown filters - #8264
feat(resources): browsable room finder with availability and dropdown filters#8264Rikdekker wants to merge 4 commits into
Conversation
Codecov Report❌ Patch coverage is
📢 Thoughts on this report? Let us know! |
bd9b34d to
3edf2ae
Compare
|
Hi @Rikdekker Thank you for the PR. I will review it as soon as I can. |
|
Hello there, We hope that the review process is going smooth and is helpful for you. We want to ensure your pull request is reviewed to your satisfaction. If you have a moment, our community management team would very much appreciate your feedback on your experience with this PR review process. Your feedback is valuable to us as we continuously strive to improve our community developer experience. Please take a moment to complete our short survey by clicking on the following link: https://cloud.nextcloud.com/apps/forms/s/i9Ago4EQRZ7TWxjfmeEpPkf6 Thank you for contributing to Nextcloud and we hope to hear from you soon! (If you believe you should not receive this message, you can add yourself to the blocklist.) |
|
Thank you for working on this improvement. I work for a non-profit educational and social services provider in Germany. We are currently evaluating RoomVox in our managed Nextcloud Enterprise environment, and this browsable room finder would make room booking significantly more intuitive and useful for our staff. As we cannot apply custom patches to the Calendar app in our Enterprise environment, we would highly appreciate seeing this functionality reviewed and, if accepted, included in an official Nextcloud Calendar release. Many thanks for your work on this. |
|
Sorry – just a quick follow-up: has there been any progress on this matter? |
|
|
Thank you for your PR, could you please use our CSS variables for styling https://docs.nextcloud.com/server/latest/developer_manual/html_css_design/css.html |
|
Thanks for the review @GVodyanov! I've pushed
The only raw |
| } | ||
| interface RoomPrincipal { |
There was a problem hiding this comment.
All interfaces should be separate files, so they can be reused
There was a problem hiding this comment.
Partially done in 3ec5a62. During a full review, I will check what interfaces can be really reused and which are only internal to a component (e.g., only used internally or used to type the component interface (props, emits, etc.)).
|
Hi @Rikdekker, I took over reviewing this and the related PR. We very much like the improvement in browsing rooms. I am currently collecting input on how we can integrate this feature in a way that aligns with the current Nextcloud design language and upcoming features in the editor interface. |
|
Hi @Rikdekker We prepared the design #8718 as something, we would directly incorporate into Nextcloud. Some nice to haves, but not required, would be:
This all would make it easier test and maybe reusable in other places. With regards to #8263 (comment) :
Look out for:
|
The "Show rooms" dialog listed every room in a flat table with a button per row to check its availability. With more than a handful of rooms that is not browsable: there is no way to narrow down by building, capacity or features, and no indication of what is free. Replace its contents with a room browser: rooms grouped per building, each card showing capacity, room number and whether the room is free for the time range of the event. Filters for building, floor, minimum capacity and features narrow the list down. A room is picked by clicking its card; nothing is written to the event until "Done" is pressed, so closing the dialog any other way discards the choice. The editor view itself is untouched: the quick search, the list of already selected rooms and resources, and the suggestions all stay as they are. Only the contents of the dialog changed. Search and filter logic lives in plain TypeScript (utils/roomFilter.ts) and is wrapped in a useRoomFilter composable, so it is unit tested without mounting a component and can be reused elsewhere. Two things are derived in the frontend on purpose, per review feedback on nextcloud#8263: - The building name is the first segment of the building address. Rooms carry no building name of their own, so this is a heuristic, kept out of the principal model. - The LOCATION value is built here rather than taken from the dav principal, whose roomAddress joins room number, story and address in that order and returns an empty string rather than null when unset. Per-card access to the free/busy timeline is kept, so the availability detail the old dialog offered is not lost. Assisted-by: ClaudeCode:claude-opus-5 Signed-off-by: Rikdekker <Rikdekker@users.noreply.github.com>
Labels sat inside the text fields but above the selects, so the filter row lined up on neither the labels nor the field boxes, and the controls had different heights. Give every filter the same shape: a label above its control, in a grid that reflows by available width. Matches the design in nextcloud#8718, which labels its filters the same way and leaves the search field to its placeholder. Also collapse the search input of a closed select. vue-select keeps it next to the selected value, where it has nothing to type into and only leaves a stray caret; it regains its width when the dropdown opens. Assisted-by: ClaudeCode:claude-opus-5 Signed-off-by: Rikdekker <Rikdekker@users.noreply.github.com>
Two problems with real imported room data, where 19 of 113 rooms have "1098 XG, Amsterdam" as their entire building address: The building name was the first address segment, which for those rooms is the postal code, so the room browser showed a group headed "1098 XG". Skip postal codes when deriving the name: a room with a building keeps its building, one without degrades to the street, and one with neither degrades to the city. The LOCATION value pulled the first segment out as the building and put the rest in front of it. With the same data that produced "Amsterdam (Amsterdam, Room 0.01)". Keep the address in the order the backend published it and append the room number, so nothing has to be guessed about what each segment means. Also stop the filter row overflowing the dialog: NcSelect defaults to a min-width of 260px, which does not fit four filters side by side. Assisted-by: ClaudeCode:claude-opus-5 Signed-off-by: Rikdekker <Rikdekker@users.noreply.github.com>
The filter row lined up on neither the labels nor the field boxes: a text field puts its label inside the border, a select puts it above. Give every filter the same shape — a label above its control — and lay them out two per row, which is what fits: a select keeps a min-width of 260px, so four side by side ran past the edge of the dialog. Follows the filter layout of the design in nextcloud#8718. Also join the meta line of a card into one string. It was assembled from separate parts with a generated separator, which ended up in the accessibility tree as a stray character for every card of a collapsed building. Assisted-by: ClaudeCode:claude-opus-5 Signed-off-by: Rikdekker <Rikdekker@users.noreply.github.com>
590067c to
9fe285d
Compare
|
@odzhychko Reworked on the new design and force-pushed. Description updated. All five "look out for" points are in: modal contents only, rooms only, Note the branch was rebuilt on current main rather than rebased, so your three Two things worth your opinion:
🤖 AI (if applicable)
|
|
The failing php/psalm checks are unrelated to this PR — it changes no PHP at |
Addresses are stored as a fixed four part format "Building, Street, PostalCode, City" so the room editor can split them back apart. That is an internal storage detail, but it was published to CalDAV clients verbatim, so a room imported without a building or street reached Nextcloud Calendar, Outlook and Apple Calendar as ", , 1098 XG, Amsterdam". Empty positions are now dropped at the publishing boundary, in both the building address and the room description. Storage is untouched, so the editor keeps its four separate fields. This gets more visible with the room browser in nextcloud/calendar#8264, which groups rooms by the first segment of the building address: without this, 95 of the 113 rooms on our test instance group under a postal code. Adds tests/Unit/Connector/RoomMetadataTest.php, which also pins down that the floor is published as room-building-story and not under the non-standard room-building-floor key it used before 1.2.2. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
|
Some context on the building-name heuristic that may be useful for reviewing it — it does not change the code here. The heuristic stays as agreed. Worth knowing though that it is a fallback rather than the only route: room backends can supply a building name directly. RoomVox 1.3.0 (released today) now publishes So for rooms from a backend that fills those properties, grouping and floor filtering run on real data; for everything else the heuristic here keeps doing its job. That is why skipping postal-code segments still matters — it is exactly the case where a backend gives us nothing better to work with. There is no Separately, @osm-frasch above mentioned evaluating RoomVox and not being able to patch Calendar in their Enterprise environment — 1.3.0 is on the App Store now, so the metadata half of what they need no longer requires a patch. The room browser in this PR is the other half. |
|
@Rikdekker Thx for reconciling the mockup and factoring out (incl. testing) filtering. |
Reworked on top of the design in #8718, against current main.
The "Show rooms" dialog listed every room in a flat table with a button per row
to check its availability. With more than a handful of rooms that is not
browsable: no way to narrow down by building, capacity or features, and no
indication of what is free.
The dialog now contains a room browser: rooms grouped per building, each card
showing capacity, room number and whether the room is free for the time range
of the event. Filters for building, floor, minimum capacity and features narrow
the list down. A room is picked by clicking its card; nothing is written to the
event until "Done" is pressed, so closing the dialog any other way discards the
choice.
Scope, as agreed in your comment
selected rooms and resources, and the suggestions are untouched — the diff on
ResourceList.vueis four lines added, five removed.AvatarParticipationStatus.vue; feat(editor): check resource availability while editing #8589 stays as it is.Structure
Search and filter logic is plain TypeScript in
src/utils/roomFilter.ts,wrapped in a
useRoomFilter(allRooms)composable, so it is unit tested withoutmounting a component. Types live in
src/types/models/roomFilter.ts.Two things derived in the frontend on purpose
Both per your feedback on #8263:
address. Rooms carry no building name, so this is a heuristic, deliberately
kept out of the principal model. Skipping postal codes matters in practice: on
our test instance 19 of 113 rooms have
1098 XG, Amsterdamas their entireaddress, which would otherwise group them under "1098 XG".
roomAddress, which joinsroom number, story and address in that order and returns an empty string
rather than
nullwhen unset.One deviation from the design
Each card keeps a small action that opens the existing free/busy timeline
("Find a time"). The design drops it, but it is the one thing today's dialog
does that the browser would otherwise lose. Happy to remove it if you would
rather keep step one strictly to the mockup.
Naming
The dialog is titled "Rooms" rather than "Rooms and resources", to match the
rooms-only scope. When the Resources tab lands, the dialog title, the "Show
rooms" button and the "Resources" section heading above it should be aligned in
one go — that heading already covers both rooms and resources today.
Tests
48 unit tests across
roomFilter,useRoomFilterandattendee.npm run ts:checkreports the same 47 pre-existing errors as clean main — none in thenew files. eslint and stylelint clean.
Verified manually on a Nextcloud 34 instance with 113 rooms from a real import:
filtering, grouping, selection, Done, LOCATION, round-tripping the selection,
re-checking availability on a time change, and the free/busy timeline.
Split out of #7996 as suggested by @nimishavijay; the principal mapping it
depended on landed separately in #8693.
🤖 AI (if applicable)